Fix #15494: Handle non-specialized functions in EtaReduce.#15498
Merged
odersky merged 1 commit intoscala:mainfrom Jun 22, 2022
Merged
Fix #15494: Handle non-specialized functions in EtaReduce.#15498odersky merged 1 commit intoscala:mainfrom
odersky merged 1 commit intoscala:mainfrom
Conversation
In Scala.js, function specialization is disabled. This means that non-specialized functions can reach `EtaReduce`. Previously, it did not handle the shape of right-hand-side for non-specialized functions returning primitives. We fix the issue by handling those, like we already handled `.asInstanceOf`s.
Contributor
|
As it is a regression from 3.1.2, I think we should backport it to 3.2.0. |
Member
Author
|
What's the procedure for that? Do I retarget this PR to |
Contributor
We merge to main and then backport. |
odersky
approved these changes
Jun 22, 2022
Contributor
odersky
left a comment
There was a problem hiding this comment.
Nice explanation of the fix!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In Scala.js, function specialization is disabled. This means that non-specialized functions can reach
EtaReduce. Previously, it did not handle the shape of right-hand-side for non-specialized functions returning primitives. We fix the issue by handling those, like we already handled.asInstanceOfs.How I fixed it
First, I took the reproduction from the issue and put it in
sandbox/scalajs/src/hello.scala. I verified that I could reproduce withNext, I looked at the Scala.js IR produced for this code:
showed nothing wrong, but
showed, sure enough, an extra wrapping when calling
asFunction0forf2andf3, which is not present forf1:Now I needed to diagnose what phase within the compiler caused the issue. I do that with
flattenis always a good first candidate when working on Scala.js issues, as it shows the dotc trees right before the back-end.flattenshows that the wrappings are already there, so the issue must come earlier. There's not supposed to be major differences in transformations in earlier phases for Scala.js versus Scala/JVM, especially when no Scala.js-specific code is involved.So I started investigating the difference in
-Xprintbetween Scala/JVM and Scala.js for the same snippet.I reached the conclusion that the group of
etaReducereceived as input something worse on Scala.js than Scala/JVM. And for good reason: the input on Scala/JVM uses specialized function types, which don't exist on Scala.js.I then tried reproducing on Scala/JVM by manually deactivating the
SpecializeFunctionsphase. Sure enough, the issue was triggered.From there, it was a matter of looking at the shape of trees received by
EtaReducewhenSpecializeFunctionsis disabled, and adding cases inEtaReduceto handle those shapes.